Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the RISC-V architecture #53508

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

aaronfranke
Copy link
Member

Implements godotengine/godot-proposals#3374 on master. Only Clang Debug builds work on RISC-V right now.

# It's also only relevant for tools build and desktop platforms,
# as doing lightmap generation and denoising on Android or HTML5
# would be a bit far-fetched.
desktop_platforms = ["linuxbsd", "osx", "windows"]
return env["tools"] and platform in desktop_platforms and env["bits"] == "64" and env["arch"] != "arm64"
supported_arch = env["bits"] == "64" and env["arch"] != "arm64" and not env["arch"].startswith("rv")
Copy link
Contributor

@AaronRecord AaronRecord Oct 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this

Suggested change
supported_arch = env["bits"] == "64" and env["arch"] != "arm64" and not env["arch"].startswith("rv")
supported_arch = env["bits"] == "64" and env["arch"] != "arm64" and env["arch"] != "rv64"

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future we might want to allow specifying a custom -march string, like "rv64gcv". There's not a lot of use for this at the moment so I didn't bother supporting it in the SConstruct file, but the startswith check is a simple improvement over checking string inequality that will work for that case. Plus it's possible that someone would want to make a 32-bit or 128-bit version, and startswith also works for that case with no downsides.

@@ -1,6 +1,7 @@
import os
import platform
import sys
from platform import machine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, but since we import platform already, I'd prefer simply using platform.machine() instead of re-importing it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this was just a blind copy from SConstruct so I missed that we already import platform.

Supports RV64GC (RISC-V 64-bit with general-purpose and compressed-instruction extensions)
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants